Remove assumption that there is always a "Username" and "Password" field #19
Conversation
Current behaviour tries to pull the "Username" and "Password" fields out of the data returned from Vault and sticks them in this custom "Credentials" struct. This is unnessecary and breaks usage in cases where the returned secret does not have those fields. (For example with the AWS secrets engine, you get "access_key" and "secret_key").
|
Hey, thanks this looks pretty useful, we don't currently use the aws vault stuff, we've got https://github.com/uswitch/kiam for that, but I definitely see the value in expanding this to handle more generic secrets. We're gonna look at using this for vault generated certs as well in the future. Your changes looks good to me, but due to our shocking lack of tests in this repo it's hard to be certain so I think I shall go and write some first. |
|
Haha sounds good! I have very minor experience with Go so would definitely recommend testing out my changes first :D |
|
Agreed- it's nice to offer the more general access to secrets rather than exclusively database credentials. My only comment/concern is that I feel like the former is probably sufficient and better. |
|
But yeah, tests to prove behaviour would be good to add @Joseph-Irving 😄 |
|
Hey sorry for taking so long, done some refactoring work to this project to make it a bit more atomic. My main concern with this change is how big a breaking change this is, even with a major version change converting all your configmaps/templates etc from .Username to .username would be a giant pain, it would be a lot nicer if we could instead support both. |
Current behaviour tries to pull the "Username" and "Password" fields
out of the data returned from Vault and sticks them in this custom
"Credentials" struct. This is unnessecary and breaks usage in cases
where the returned secret does not have those fields. (For example
with the AWS secrets engine, you get "access_key" and "secret_key").